Add default value option #352
Conversation
ahmednaguib
force-pushed
the
add-default-value-option
branch
from
a057d16
to
91dc6c9
Compare
| return unless valid?(entity) | ||
|
|
||
| output = value(entity, options) | ||
| output.blank? && @default_value.present? ? @default_value : output |
There was a problem hiding this comment.
I am not a maintainer, but I stumbled across this PR while I was looking for a sane default ability for entities.
blank? and present? are Rails specific, so to keep Grape framework agnostic you might want to shuffle this around. In fact, the blank? check replaces some valid options like false and [] with the default value, which I would not expect.
| output.blank? && @default_value.present? ? @default_value : output | |
| output.nil? ? @default_value : output |
I'm not sure the .present? check bought you anything anyway if we keep it strictly to a nil check.
There was a problem hiding this comment.
.blank? and .present? are part of ActiveSupport and this is included in the Grap gemspec, so it can be used without adding additional dependencies
but suggest to move the getting of the value into the same named method → value()
There was a problem hiding this comment.
@LeFnord not sure if I get this part:
but suggest to move the getting of the value into the same named method → value()
There was a problem hiding this comment.
@LeFnord Do you still consider the behavior of blank? to be appropriate though? If I had a boolean attribute with a default of true, then the above implementation will always return true no matter what, as:
false.blank?
# => trueIt seems to me like the only sane value to override with a default would be nil
There was a problem hiding this comment.
@dlinch … I'm only saying, that is no problem to use it, cause it is in place
and yeap would prefer your suggestion but with a check on blank, instead only nil
output.blank? ? @default_value.presence : output
|
sorry @ahmednaguib for the late reply |
Description
I noticed there is no direct way of defining default value for
exposewhen a field is returningnil. Of course there is a way to do it via a block but would be a nice option to have it like this:expose :x, default: 0I also saw there is an issue opened #327 asking for the feature so I made the pr maybe it will be useful for other folks.